Skip to content

Consolidate integration to one Connection screen - #102265

Open
Krishna2323 wants to merge 54 commits into
Expensify:mainfrom
Krishna2323:krishna2323/101509-connections-page
Open

Krishna2323 wants to merge 54 commits into
Expensify:mainfrom
Krishna2323:krishna2323/101509-connections-page

Conversation

@Krishna2323

@Krishna2323 Krishna2323 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

Adds a unified Connections page that combines Accounting, HR, Recruiting, Receipt partners and MCP. It sits behind the unifiedConnections beta.

  • Beta on:
    • The workspace menu shows a single Connections item, with tabs for each category and a search box.
    • Each connected integration's settings open in a right-hand panel.
    • The old Accounting, HR, Recruiting, Receipt partners and MCP routes render Connections, so existing links and deep links still work.
    • Connecting a provider turns the matching workspace feature on automatically, so those toggles are removed from More features.
    • A one-time tooltip points at the new menu item.
  • Beta off: the old pages, menu items and More features toggles work exactly as they do on main.

Fixed Issues

$ #101509
PROPOSAL

Tests

Setup:

  1. Enable the unifiedConnections beta for your account. On a dev or staging build, you can also go to Account > Troubleshoot > Beta overrides and turn on unifiedConnections. Without the beta, none of the steps below show up.

  2. To see the Recruiting tab, also enable the recruiting beta.

  3. Use a Control workspace where you're an admin. Connect QuickBooks Online (or Xero) to it, plus one HR provider (for example BambooHR) and Uber for Business.

  4. Menu and tooltip

    1. Open the workspace. Check that the left menu shows a single "Connections" item. There should be no separate Accounting, HR, Receipt partners or MCP items.
    2. Check that a tooltip reading "New All your connections in one place" points at Connections. "New" should be bold and green.
    3. Click Connections. Check that the tooltip disappears.
    4. Reload the page. Check that the tooltip doesn't come back.
    5. With a new account, open a workspace and let the tooltip show. Then open a different menu item, such as Members. Check that the tooltip is dismissed and doesn't come back after a reload.
  5. Page layout

    1. Check the order on the Connections page: connected integrations first, then the tabs All, Accounting, HR & People, Recruiting, Receipts and AI & MCP, then the search box, then the listings, then the "Don't see yours? Suggest an integration, we'll look into it. >" footer.
    2. Check that each connected card shows an "Active" or "Broken" badge and a sync message. It should have a Configure button, or a red Fix button if it's broken.
    3. Switch between the tabs. Check that each one shows the matching listings, minus any integrations that are already connected.
    4. Before connecting Uber for Business (or after disconnecting it), check that its card shows an "Offer" badge.
    5. Open a Configure panel and close it. Check that the tab you had selected is still selected.
    6. Leave the Connections page and come back. Check that it opens on the All tab.
    7. Click the footer. Check that Concierge opens.
  6. Search

    1. Type "net". Check that NetSuite appears.
    2. Type "zzzz". Check that the "No results found" empty state appears and the footer is hidden. The search box shouldn't change width.
    3. Clear the search. Check that the active tab's listings and the footer come back.
  7. Responsive layout: resize the browser across the 1024px mark.

    1. At 1024px and wider, check that the cards sit two per row and the search box sits to the right of the tabs.
    2. Below 1024px, check that each card takes the full width and the search box moves to its own full-width row below the tabs.
  8. Replace warning

    1. With QBO connected, click "+" on Xero.
    2. Check that a modal appears with the title "Replace connection?", the message "This will remove your current QuickBooks Online connection." and a red "Replace" button.
    3. Click Cancel. Check that nothing changes.
    4. With BambooHR connected, click "+" on another HR provider. Check that the same "Replace connection?" prompt appears.
  9. Connect flows: click "+" on Claude or ChatGPT. Check that the connect link opens in a new tab.

  10. Configure panels: click Configure on each of QBO, BambooHR and Uber.

    1. Check that a right-hand panel opens. Its header should show a round logo, the integration name, the sync status and a ⋮ menu.
    2. Check that the rows below it (Import, Export, Advanced and so on) span the panel edge to edge, with hover highlights that don't overflow.
    3. Check that the ⋮ menu offers Sync now and Disconnect.
    4. Click Sync now. Check that a spinner replaces ⋮ while it syncs.
    5. Disconnect the integration. Check that you're taken back to Connections and the card moves into the listings.
  11. More features

    1. Open More features. Check that there are no toggles for Accounting, Receipt partners, HR or MCP.
    2. Check that the other toggles still work.
  12. Permissions

    1. Log in as an Auditor on the workspace. Check that Connections appears in the menu and the page opens rather than showing Not Found.
    2. Click "+" on any listing. Check that the read-only modal appears instead of a connect flow.
    3. As the Auditor, open a direct Xero or Uber offer link. Check that the auditor can't connect from it.
  13. Onboarding: create a new account and choose the accounting-software option. Check that onboarding works exactly as it does on main.

  14. Old routes: paste an old URL such as /workspaces/<policyID>/accounting, /hr or /receipt-partners. Check that the Connections page opens.

  15. Beta off: turn off unifiedConnections and reload.

    1. Check that the menu shows the separate Accounting, HR, Receipt partners and MCP items again, with no Connections item.
    2. Check that those pages and the More features toggles work exactly as they do on main.

Mobile (iOS and Android):

  • Repeat steps 1–3 and step 7.

  • Check that the back button on the Connections page and on each panel returns to the previous screen.

  • Check that the footer's ">" lines up with the text.

  • Verify that no errors appear in the JS console

Offline tests

  1. Go offline. Check that the "+" on unconnected cards is disabled.
  2. Check that the Configure panels still open.

QA Steps

  • Same as tests. Make sure the unifiedConnections beta is enabled on the test account first.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I verified there are no new alerts related to the canBeMissing param for useOnyx
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
android_hybrid.mp4
Android: mWeb Chrome
android_mWeb.mp4
iOS: Native
ios_hybrid.mp4
iOS: mWeb Safari
ios_mWeb.mp4
MacOS: Chrome / Safari
Monosnap.screencast.2026-09-29.12-42-42.mp4

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@melvin-bot

melvin-bot Bot commented Sep 25, 2026

Copy link
Copy Markdown

Hey, I noticed you changed src/languages/en.ts in a PR from a fork. For security reasons, translations are not generated automatically for PRs from forks.

If you want to automatically generate translations for other locales, an Expensify employee will have to:

  1. Look at the code and make sure there are no malicious changes.
  2. Run the Generate static translations GitHub workflow. If you have write access and the K2 extension, you can simply click: [this button]

Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running:

npx bun ./scripts/generateTranslations.ts --help

Typically, you'd want to translate only what you changed by running npx bun ./scripts/generateTranslations.ts --compare-ref main

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@Krishna2323
Krishna2323 marked this pull request as ready for review September 25, 2026 17:01
@Krishna2323
Krishna2323 requested review from a team as code owners September 25, 2026 17:01
@Krishna2323
Krishna2323 marked this pull request as draft September 25, 2026 17:01
@melvin-bot
melvin-bot Bot requested review from JmillsExpensify and situchan and removed request for a team September 25, 2026 17:01
@melvin-bot

melvin-bot Bot commented Sep 25, 2026

Copy link
Copy Markdown

@situchan Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot removed the request for review from a team September 25, 2026 17:01
@Krishna2323

Copy link
Copy Markdown
Contributor Author

Sorry, marked it as ready by mistake. It's still in progress.

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…ltip

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@Krishna2323 Krishna2323 changed the title Krishna2323/101509 connections page Consolidate integration to one Connection screen Sep 29, 2026
@situchan

situchan commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Please check below findings:

A connection that only works in Expensify Classic doesn't show up on Connections. useAccountingConnectionListings.ts:74 only looks at New Expensify integrations. On main, the Accounting page hides the list of integrations to connect and shows "Go to Expensify Classic" or an error instead. The Configure panel copied that check (ConnectionsAccountingPage.tsx:150), but you can only open the panel from a connected card, and no card appears here.

  1. Use a workspace connected to an integration that only works in Expensify Classic.
  2. Go to Workspace > Connections.
  3. Notice that there's no connected card, and every accounting provider has a "+".
  4. Click Xero. Notice that setup starts with no Replace prompt.

Saving a Final approver from the HR Configure panel closes the panel. The Final approver pages still go back to the old HR page: GustoFinalApproverPage.tsx:30, ZenefitsFinalApproverPage.tsx:30 and MergeHRFinalApproverPage.tsx:31. I found this by reading the code and haven't tried it in the app.

  1. On a Control workspace with Gusto connected, go to Workspace > Connections.
  2. Click Configure on Gusto.
  3. Make sure Approval mode isn't Custom, then click Final approver.
  4. Pick a member.
  5. Expected: you're back on the Gusto Configure panel. Actual: the panel closes, the URL ends in /hr, and the tab resets to All. The back button does the same.

The onboarding accounting task says "Click Accounting".
Beta users have no Accounting menu item (en.ts:3719). The link still works because /accounting shows Connections, but the written steps don't match.
The issue says onboarding shouldn't change, so please confirm this is OK while the beta is on.

@MelvinBot

Copy link
Copy Markdown
Contributor

Regression review

Beta off: I found no regressions. Every changed path either falls back to the old value or adds a new argument with a default that keeps current behavior. That includes the shared TabSelectorBase and MenuItem changes.

Beta on: I found 2 navigation regressions and 3 behavior changes. These come from reading the code at c333a7d. I didn't reproduce them in a browser, because they need a workspace with connected integrations.

Regressions

  1. Back from some accounting settings pages closes the Configure panel. The QBO Export, Xero Export, Sage Export, QBD Advanced, and Certinia Export/Advanced handlers build their route from the old /accounting base. Back resolves to /accounting and closes the right-hand panel. Then it swaps the Connections page for the old route.

  2. HR/Recruiting sync results can open Not Found. useMergeSyncResultsPage.ts:57 adds the results suffix to the active route. When Connections renders under an old route such as /accounting or /receipt-partners, that screen isn't in the entryScreens list at ROUTES.ts:1315. The path then falls through to Not Found. Many existing links still point to /accounting, including Fix connection, onboarding tasks, and empty states. This is medium-low confidence.

situchan's Not Found report (beta turned off while on /connections) is confirmed. WorkspaceConnectionsPage.tsx:369 blocks the page without redirecting to the old page. The same happens when a user without the beta opens a shared /connections link.

Behavior changes to confirm

  • Features turn on before a connection succeeds, and can't be turned off. AccountingContext/index.tsx:161-171 enables Connections before the Control upgrade check. If you abandon the upgrade, the feature stays on. HR, Uber, and MCP clicks work the same way, and the More features toggles are hidden for beta users. MCP has no disconnect state, so beta admins can't turn it off.
  • Uber "+" may do nothing. useReceiptPartnerConnectionListings.ts:57-60 returns silently when connectFormData is missing. On main this button was only reachable with Receipt partners turned on. If the API leaves this data out when the feature is off, the click shows nothing.
  • The tooltip also shows for new workspaces. TOOLTIPS.ts:118-124 has no "existing users only" check. This matches trjExpensify's question about the tooltip on a new trial account.
Areas checked and found safe
  • With the beta off: menu items, sidebar focus and access, More features toggles, Policy.ts enable functions (shouldGoBack defaults to true), the upgrade page, ClaimOfferPage, routes (only additions, no signature changes), and the pages wrapped in withUnifiedConnectionsBeta.
  • Shared code: TabSelectorBase (the fade only renders with shouldShowScrollFade), TabSelectorItem, MenuItem (new props are off by default), useSearchResults (4th tuple element, callers use 3), useDynamicForwardPath, focus trap list, and styles.
  • Parity of the new panel with the old Accounting page: sync errors, credential and reconnect actions, entity and company selectors, card reconciliation, offline and read-only handling, and the replace flow are all present.

Next Steps: Reply with @MelvinBot <your feedback> to refine this review.


view run

@MelvinBot

MelvinBot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Web test steps verification results: ⚠️ partial

With the beta off, More features and the Create expense tabs behave as on main. With the beta on, the single Connections item, the Connections page, and the Uber panel work. Clicking "+" on Claude moved the test browser off the app, so steps 7 and 8 couldn't finish.
Browser on claude.ai after clicking + on Claude

Step Status
1. Beta off: More features shows the Integrate togglesThe Integrate section shows Accounting, HR, Receipt partners, and MCP toggles.
More features, beta off, Integrate toggles
✅
2. Beta off: Create expense tabs render and switch normally (shared TabSelector change)Switching between Manual and Scan works. Distance is a separate quick action in this build, which is unrelated to this PR.
Create expense Manual tabCreate expense Scan tab
✅
3. Beta on: one Connections menu item, and no Integrate toggles in More featuresThe menu shows Connections and no separate Accounting, HR, or MCP items. More features has no Integrate section.
Workspace menu, beta on, single Connections itemMore features, beta on, no Integrate section
✅
4. Beta on: the "New: All your connections in one place" tooltip on a brand-new accountNo tooltip appeared on Connections after several visits. So the "tooltip shows for new workspaces" concern from my code review didn't reproduce here.
Workspace menu, beta on, no tooltip
❌
5. Beta on: /workspaces/<policyID>/accounting renders ConnectionsThe tester didn't enter the URL directly, because a reload risks losing the session. Opening Connections from the menu rendered the page with its tabs.
Connections page, beta on
⚠️
6. Beta on: "+" on Uber for BusinessAn in-app panel opened with the 5% offer and a "Connect to Uber for Business" button. This isn't a silent no-op.
Uber for Business connect panel
✅
7. Beta on: "+" on Claude, then check whether any UI can turn MCP back offThe test browser moved to claude.ai in the same window, and the session ended. This is likely a quirk of the headless browser rather than a PR bug. useMCPConnectionListings.ts:51 uses the same openExternalLink as the old MCP page on main. The tester couldn't check the MCP turn-off part.
Browser on claude.ai after clicking + on Claude
❌
8. Turn the beta off while on /connections, then return to that URLNot tested, because the session had already left the app in step 7.
❌

view run · view recording

@JmillsExpensify

Copy link
Copy Markdown
Contributor

@Krishna2323 we're aiming for early next week on this one when you have a second to get back to it.

@Krishna2323

Copy link
Copy Markdown
Contributor Author

Working on it, should be ready again in few moments.

@Krishna2323

Copy link
Copy Markdown
Contributor Author

When beta on, Accounting page has multiple routes to access. (/connections, /accounting)
So this leads to a bug of not found page when turn off beta while on /connections route

This only happens when the beta is turned off while on /connections, or when someone without the beta opens a /connections link. Once the beta is removed, /connections will be the only route and the old pages go away, so I'd rather not add a temporary fallback. Happy to add one if you think it's needed before then.

@Krishna2323

Krishna2323 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Was "Need help connecting? Read our guide." intentionally or accidentally removed in new MCP page?

With the beta on, the MCP page is replaced by Connections, which doesn't have the "Need help connecting? Read our guide." row. @dubielzyk-expensify should I add it back under the grid on the AI tab?

Tooltip on a new trial account.

@dubielzyk-expensify @trjExpensify right now the "All your connections in one place" tooltip shows to everyone with the beta, including brand-new accounts. We dropped the cutoff date because the beta rollout makes any fixed date wrong. One option is to show it only on workspaces that already used Accounting, HR, Recruiting, Receipt partners or MCP before. Is that the right rule, or should "existing user" mean the account rather than the workspace?

@Krishna2323

Copy link
Copy Markdown
Contributor Author

MCP can't be turned off

@MelvinBot With the beta on, More features hides the Integrate toggles, and MCP has no connected state on Connections. So admins have no way to turn MCP off, and clicking "+" on Claude turns it back on if it was off. @dubielzyk-expensify should we keep the MCP toggle in More features for beta users, or add a Configure panel on the MCP card with an on/off switch?

@situchan,

  1. Classic-only connection: this isn't reachable. SUPPORTED_ONLY_ON_OLDDOT is empty on main too, so main's "Go to Expensify Classic" branch never runs, and Connections behaves the same.
  2. Final approver: fixed, it now goes back to the Configure panel.
  3. Onboarding "Click Accounting": the link still works, because /accounting shows Connections when the beta is on. The issue says onboarding shouldn't change, so I left it. @trjExpensify OK to keep it as is while the beta is on?

Fixed items:

  • Back from settings pages: fixed, including the Xero, Sage, QBD and Certinia export sub-pages. They now resolve from the current route, like QBO and NetSuite.
  • Sync results Not Found: fixed, the old integration routes are now allowed entry screens.
  • Features turning on before connecting: intended. Accounting has to turn on before the Control upgrade so Collect workspaces can connect NetSuite, and HR and Uber follow the same pattern. The MCP gap is covered in my question above.
  • Landscape tooltip: fixed. The tooltip now hides while Connections is off screen and shows again when you scroll to it.

@Krishna2323

Copy link
Copy Markdown
Contributor Author

@MelvinBot review

@trjExpensify

trjExpensify commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

With the beta on, the MCP page is replaced by Connections, which doesn't have the "Need help connecting? Read our guide." row. @dubielzyk should I add it back under the grid on the AI tab?

I think it would be cool to add it back to the AI tab. 👍

@MelvinBot

Copy link
Copy Markdown
Contributor

None of the fixes you listed are on the branch yet. The head is still 01642d3, which only merges main, and nothing after c333a7d touches these files. Can you push the fix commit? Then I'll re-check it.

What's still in the code at 01642d3:

  • Back from settings pages: QBO, Xero, Sage, QBD Advanced and Certinia still build on /accounting (utils.tsx:98, 358, 400, 482-484). QBO Export uses this base too, so only NetSuite and QBD Export follow the current route today. When you fix it, also check the export subpages that go back to /accounting. Examples are the DynamicXero*, DynamicSageIntacct* and DynamicQuickbooksDesktopExportPage pages, plus CertiniaExportPage.tsx:126.
  • Sync results Not Found: ROUTES.ts:1315 and 1321 still leave out ACCOUNTING.ROOT, RECEIPT_PARTNERS and MCP.
  • Final approver: the Gusto, Zenefits and Merge HR final approver pages still go back to WORKSPACE_HR (MergeHRFinalApproverPage.tsx:31), not to the Configure panel.
  • Landscape tooltip: the menu still relies on shouldHideOnScroll (WorkspaceInitialPage.tsx:341-348). Its ScrollView never emits CONST.EVENTS.SCROLLING, so the tooltip never re-checks its position. InitialSettingsPage.tsx:132 shows the useScrollEventEmitter wiring it needs.

MCP: I confirmed the gap is still open, so the question to design stands. The only way to turn MCP off is the Integrate toggle in More features (WorkspaceMoreFeaturesPage/index.tsx:372), and it's hidden when the beta is on.

Features turning on early: that reasoning works for Accounting and HR, so I'm fine leaving them as they are.


view run

@trjExpensify

Copy link
Copy Markdown
Contributor

@dubielzyk-expensify @trjExpensify right now the "All your connections in one place" tooltip shows to everyone with the beta, including brand-new accounts. We dropped the cutoff date because the beta rollout makes any fixed date wrong. One option is to show it only on workspaces that already used Accounting, HR, Recruiting, Receipt partners or MCP before. Is that the right rule, or should "existing user" mean the account rather than the workspace?

That might work. I take it you're talking about the areXenabled: true flags? The other thing I thought about was maybe looking at workspaces that have a connection in policy.connections

Classic-only connection: this isn't reachable. SUPPORTED_ONLY_ON_OLDDOT is empty on main too, so main's "Go to Expensify Classic" branch never runs, and Connections behaves the same.

I don't think we have any OldDot only connections at this point since we finished building Certina, so I'm not sure if this is necessary anymore?

Onboarding "Click Accounting": the link still works, because /accounting shows Connections when the beta is on. The issue says onboarding shouldn't change, so I left it. @trjExpensify OK to keep it as is while the beta is on?

Yeah, what we meant by this is is that we should still advertise "Accounting", "HR" etc. But before, disabling Accounting would disable Accounting in the workspace - and that's obviously not a thing now. 👍

@MelvinBot

Copy link
Copy Markdown
Contributor

The branch hasn't changed since my last review. The head is still 01642d3, so the findings there still apply. Push the fix commit and tag me again, and I'll re-check it.


view run

@Krishna2323

Copy link
Copy Markdown
Contributor Author

@MelvinBot pushed the changes, please check again

…nnected workspaces

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@MelvinBot

Copy link
Copy Markdown
Contributor

I re-checked the branch at eb206d8. All four fixes are in, and I found no new beta-off regressions. Two smaller items are left.

Fixed:

  • Back from settings pages. QBO, Xero, Sage, QBD Advanced and Certinia now build on the current route. Their subpages use useDynamicBackPath. Every dynamic route you made relative lists both ACCOUNTING.ROOT and CONNECTIONS_ACCOUNTING as entry screens. With the beta off, the current route is still /accounting, so nothing changes there.
  • Sync results Not Found. Both result routes now accept the old integration screens (ROUTES.ts:1315-1340).
  • Final approver. Gusto, Zenefits and Merge HR now go back to the screen you came from.
  • Landscape tooltip. The menu ScrollView now emits scroll events, and the first display re-checks whether the anchor is on screen.

The MCP help link and the "connected workspaces only" tooltip rule (WorkspaceInitialPage.tsx:215) also look correct.

Still to check:

  1. The shared tooltip change affects other screens. BaseEducationalTooltip.tsx:165 now runs the on-screen check on first display for every shouldHideOnScroll tooltip. That includes the LHN product-training tooltip, the mileage rate tooltip and FieldRequirementSettingRow, regardless of beta. The check always reserves bottomTabHeight (72px), even on wide layouts that have no bottom tab bar. So an anchor in the bottom ~62px of the window now stays hidden. Before, it showed. This is low risk. To keep other screens unchanged, you could put this behind a prop that only the workspace menu passes.
  2. NetSuite tax and travel-billing pages still go back to /accounting. NetSuiteTaxPostingAccountSelectPage.tsx:40, NetSuiteProvincialTaxPostingAccountSelectPage.tsx:38 and NetSuiteTravelBillingConfigurationPage.tsx:44 go back to /accounting/netsuite-export. That route isn't in the stack when you start from the Configure panel. This PR doesn't touch these pages, but they have the same Back problem you just fixed elsewhere. I haven't reproduced it.

Waiting on decisions, not code: a way to turn MCP off for beta users (design question above), and the /connections Not Found when the beta is turned off. You chose not to add a fallback for that.

I only read the code for this pass. I didn't run a browser test, because these flows need connected NetSuite, QBO, Xero, Sage or Certinia workspaces.


Next Steps: Reply with @MelvinBot <your feedback> to refine this review.


view run

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@Krishna2323

Copy link
Copy Markdown
Contributor Author

@situchan ready for re-review again.

Since your last review:

  • Back from accounting settings: goes back to the Configure panel instead of /accounting. This covers QBO, Xero, Sage Intacct, QuickBooks Desktop, Certinia and NetSuite, including their export sub-pages.
  • HR final approver: goes back to the Configure panel after you pick someone.
  • Sync results: no more Not Found when a sync finishes while you're on an old page like /accounting.
  • Landscape tooltip: hidden while the Connections menu item is off screen. The change only applies to the workspace menu, so other tooltips are unchanged.
  • MCP help link: back on the AI tab.
  • Connections tooltip: now only shows on workspaces that already have a connection.

Only one item still needs a call: how admins turn MCP off when the beta is on. Waiting on @dubielzyk-expensify (question).

@dubielzyk-expensify

Copy link
Copy Markdown
Contributor

@dubielzyk-expensify should we keep the MCP toggle in More features for beta users, or add a Configure panel on the MCP card with an on/off switch?

Does the MCP toggle do anything functionally than turn the nav item on? Cause otherwise my plan for this tab was to basically list all the MCP integrations in the AI tab and then have a generic MCP tab that acts as the AI help link:

CleanShot 2026-10-09 at 1 27 39 PM@2x

Curious to hear what @trjExpensify and @Expensify/design thinks about that.

@shawnborton

Copy link
Copy Markdown
Contributor

Does the MCP toggle do anything functionally than turn the nav item on?

I think it just turns the nav item on. I'm not quite following your proposal though, I think I'm having a hard time understanding the arrows 🤣 might need a new mock!

@Krishna2323

Krishna2323 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

@dubielzyk-expensify @shawnborton The MCP toggle only shows or hides the MCP nav item. With the beta on, that item is replaced by Connections, the same as Accounting and HR, so the toggle has nothing left to control. That's why I hid it along with the other integration toggles. Unless the backend uses isMCPEnabled to block MCP access, I think we don't need an off switch.

@lakchote could you please confirm this?

@situchan situchan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No blockers left

@melvin-bot

melvin-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

We did not find an internal engineer to review this PR, trying to assign a random engineer to #101509 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

@melvin-bot
melvin-bot Bot requested a review from Gonals October 9, 2026 12:37
@MelvinBot

Copy link
Copy Markdown
Contributor

Fresh review at c35c2c9

Beta off: one minor regression. Some sub-pages now call a plain goBack(). On a deep link or refresh, Back closes the panel instead of opening the parent page. Everything else matches main, including the shared TabSelector, MenuItem and EducationalTooltip changes.

Beta on: 3 confirmed regressions and 3 that need a check. The fixes from a85c32b–c35c2c9 work: Back to the Configure panel, Final approver, sync results, the landscape tooltip and the MCP help link. I read the code only and didn't run it. These flows need workspaces with live integrations.

Regressions

Beta off and on

  1. Back skips the parent page on a deep link or refresh. These pages went back to a fixed parent route. They now call goBack(), which only pops the stack:

    Repro: refresh on …/connections/netsuite/export/tax-posting-account/select, then press Back. You land on Accounting instead of NetSuite Export. Normal in-app navigation still works. Fix: keep a fallback route that points to the panel or the old page, depending on the beta.

Beta on

  1. A failed Uber disconnect shows no error. If the API fails, uber comes back with errorFields. The old page showed a red dot for that. The new card only treats the enter-credentials error as broken (useReceiptPartnerConnectionListings.ts:75-79). The card goes back to "Active".

  2. Replacing an HR provider deletes the old one first. useMergeConnectionListings.ts:119-120 calls removePolicyConnection, then opens Merge Link. If the admin closes Merge Link, the workspace has no HR connection. It also loses its approval mode and final approver settings. The Replace modal was agreed, but this order loses data.

  3. Links into a connection's settings now open the All tab. withUnifiedConnectionsBeta.tsx renders Connections on old routes with no tab or panel selected. The user must find the card and click Configure. Affected entry points:

    • The Home "Fix" item (FixPolicyConnection)
    • The workflow "configured in " link (WorkflowUtils)
    • "Configure HR sync" on Members
    • ImportedFromAccountingSoftware and the Rules empty states

    For a broken connection, consider opening the matching panel.

Needs a check (depends on backend data or timing)

  1. Uber "+" may do nothing. useReceiptPartnerConnectionListings.ts:57-60 returns silently when connectFormData is missing. That check runs before the feature turns on. If the API only sends this data when Receipt partners is on, Uber can't be connected from Connections. ClaimOfferPage would open UBER_CONNECT_URL?undefined.
  2. An unexpected Uber invite screen may open. useReceiptPartnerConnectionListings.ts:35-45 opens the invite flow when "connected" changes from false to true. It doesn't check whether the screen is focused. If uber.enabled only arrives with the receipt partners fetch, this can fire on every cold visit to Connections.
  3. Panel deep links can redirect to Connections on a cold start. ConnectionsMergePageBase checks a stored "data fetched" flag. ConnectionsReceiptPartnersPage waits on policy.isLoading instead of isLoadingReceiptPartners. Either page can decide "not connected" before the data loads.

Code quality

  1. ConnectionsAccountingPage.tsx copies about 450 lines from PolicyAccountingPage.tsx. It copies the integration derivation, the overflow menu, the 9-integration settings switch, the menu items and the QBO expiry hint. They have already drifted: the copy dropped a wrapperStyle, and disconnect behaves differently. Every integration fix now has to land twice for as long as the beta lives. Move the shared code into a hook such as useConnectedAccountingIntegration and use it in both pages. The same applies to:

    • ConnectionsReceiptPartnersPage: about 150 lines from WorkspaceReceiptPartnersPage
    • ConnectionsMergeProviderCard: about 100 lines from MergeProviderCard, and it renders a full page, not a card
    • ConnectionsAccountingPageQBOTokenExpiryTest: nearly the same as the existing test
  2. The beta check uses 5 mechanisms across about 24 files:

    • The HOC
    • shouldBeBlocked
    • A boolean on getAccountingConnectionsRoute
    • A 15th positional argument on getAccountingIntegrationData, so callers pass runs of undefined
    • Inline checks

    Removing the beta later means finding all of them. Use one useUnifiedConnections() hook or route helper, and switch getAccountingIntegrationData to an options object.

  3. "Turn the feature on when connecting" is repeated in 5 places. Each enablePolicy* function also gained a positional shouldGoBack, so calls read enablePolicyHR(id, true, false). One helper with a named option would fix both. useAccountingConnectionListings also repeats the upgrade check that startIntegrationFlow already does.

  4. The panels both redirect and block on "not connected". useRedirectUnconnectedPanelToConnections plus shouldBeBlocked={… || !connected} appear in all three panels. Which one wins is a race. Keep one.

  5. TabSelectorBase re-renders every tab on every scroll frame. It stores offset and widths in state (TabSelectorBase.tsx:53-89). Only store canScrollLeft and canScrollRight. The SVG gradient IDs in TabSelectorScrollFade are fixed strings, so two faded selectors on one page would collide. Use useId.

  6. The empty-state text is built from fragments. WorkspaceConnectionsPage.tsx:329-331 joins a prompt, ' ', a link and noResultsPromptEnd, which is just '.'. This adds a space in Japanese and forces English word order. Use one translated string with an inline link.

  7. The tooltip logic lives inline in WorkspaceInitialPage. It adds about 30 lines of state, refs and effects, plus a custom render that repeats renderProductTrainingTooltip. Extract a useConnectionsMovedTooltip() hook.

  8. Smaller items:

    • withUnifiedConnectionsBeta casts with props as unknown as ….
    • WorkspaceConnectionsPage subscribes to all of ACCOUNT to read two fields. Add a selector.
    • ConnectionsReceiptPartnersPage reuses the old page's testID and has an unexplained exhaustive-deps disable.
    • Tests mock out AccessOrNotFoundWrapper, so the beta and not-connected gates are never tested. The listing hooks and withUnifiedConnectionsBeta have no tests.
Checked and found safe
  • Beta off:
    • The menu, sidebar focus and access, and More features (a whitespace-only diff inside the beta check).
    • Policy.ts defaults, WorkspaceUpgradePage, ClaimOfferPage (auditors are blocked), AccountingContext, and routes (entryScreens were only extended).
    • The tooltip never registers.
    • shouldCheckVisibilityOnFirstDisplay has one caller, the workspace menu, so other tooltips are unchanged.
    • The new onScroll only emits an event.
  • Beta on:
    • Access is the same for admin, editor and auditor.
    • Accounting panel parity, the Uber settings rows, and the Recruiting panel.
    • Every dynamic route opened from a panel lists that panel as an entry screen.
    • No panel sub-page still goes back to /accounting or /hr.
  • Already decided in this thread, so not repeated here: the /connections Not Found when the beta turns off, features turning on before connecting, the MCP off switch, Classic-only connections, and the onboarding copy.

Next Steps: Reply with @MelvinBot <your feedback> to refine this review.


view run

@dannymcclain

Copy link
Copy Markdown
Contributor

I'm not quite following your proposal though, I think I'm having a hard time understanding the arrows 🤣

Let me try to explain what the arrows are showing...

  • Previously, each "AI integration" had a Connect button. On the new connections page, those Connect buttons are replaced by the Plus icons in the connection cards (for ChatGPT, Claude, and Cursor)
  • On the previous MCP > Connections card, we displayed that little line at the bottom about needing help connecting, with a link to the help site (or wherever). On the new connections page, Jon has just added a generic MCP card that links to the guide

Does that make sense? 😅

image

@lakchote

lakchote commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@Krishna2323 yea confirmed, nothing in the backend uses isMCPEnabled to block MCP access. Auth and Web-E only store it, and the MCP server doesn't check it, so the toggle only controls the MCP nav item and page in App. Fine to hide it with the other integration toggles.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.